Refactor about:preferences#payments #6009

luixxiul · 2016-12-03T15:09:53Z

Submitted a ticket for my issue if one did not already exist.
Used Github auto-closing keywords in the commit message.
Added/updated tests for this change (for new code or code which already has tests).
Ran git rebase -i to squash commits (if needed).

I created the follow-up of this PR: #6047

Test Plan:
As I restyled about:preferences#payments again with #6047, please test just these two for this PR:

Refactor about:preferences#payments #6009 (comment)
- Make sure the row of the ledger table on about:payments is aligned horizontally
Refactor about:preferences#payments #6009 (comment)
- Make sure the top right button on the modal dialog works

luixxiul · 2016-12-04T06:42:30Z

less/modalOverlay.less

@@ -58,35 +58,6 @@
        overflow: hidden;
      }
    }
-
-    .qrcodeOverlay {


.qrcodeOverlay was moved under #bitcoinDashboard. Code which is not specific to the general overlay, if any other, should be moved to a proper place.

luixxiul · 2016-12-04T06:46:17Z

less/about/preferences.less

@@ -13,33 +13,6 @@ body {
  display: none !important; /* for testing/debug only */
 }

-#paymentsSwitches {


Moved under .titleBar

luixxiul · 2016-12-04T06:47:08Z

less/about/preferences.less

@@ -69,12 +42,6 @@ body {
  display: inline-block;
 }

-.bitcoinQRTitle {


Moved under .qrcodeOverlay .dialog-body

luixxiul · 2016-12-04T06:47:37Z

less/about/preferences.less

@@ -516,30 +483,6 @@ table.sortableTable {
  }
 }

-.paymentsMessage {


Moved under #paymentsContainer

luixxiul · 2016-12-04T06:48:43Z

less/about/preferences.less

@@ -563,13 +506,6 @@ table.sortableTable {
  text-align: left;
 }

-span.paymentHistoryButton.browserButton {


Moved under .walletBar .settingsList .settingItem

luixxiul · 2016-12-04T06:53:25Z

less/about/preferences.less

-      line-height: 1.3em;
+      margin: 0;
+
+      &.subTitle {


.settingsListSubTitle was replaced with .settingsListTitle .subTitle, which can be more easily maintained.

luixxiul · 2016-12-04T06:56:12Z

less/about/preferences.less

+        margin: 0 auto;
+      }
+
+      .dialog-header {


Because any modal creates .dialog-header, .dialog-body, .dialog-footer, they should be set inside it, whether they have properties or not. This will work for a guide for devs and help avoiding unexpected regressions.

luixxiul · 2016-12-04T07:12:18Z

js/about/preferences.js

@@ -1299,7 +1301,7 @@ class PaymentsTab extends ImmutableComponent {
          <span className='sectionTitle'>Brave Payments</span>
          <span className='sectionSubTitle'>beta</span>
        </div>
-        <div className='pull-left' id='paymentsSwitches'>


Removed pull-left as flexbox model was applied.

bsclifton · 2016-12-04T18:36:50Z

@luixxiul let me know when this is ready for review 😄 I can test it locally on Mac and Windows to make sure it works great

bsclifton · 2016-12-05T22:20:48Z

OK @luixxiul- I got a chance to review, most everything looks good 😄

Can you please cherry pick this commit into your branch? I had to update selectors to fix some tests. It also fixes an issue that @willy-b had opened. I was unable to push to your fork (usually, you can if you check the box on the PR screen saying "allow folks with commit access to push changes")
https://github.com/bsclifton/browser-laptop/commit/17f54098617dab5266e4992f99447fb1fd5ef38a
When viewing the table with sites, if any sites are verified (ex: clifton.io), the verified checkmark is going out of bounds.
When pulling up QR code, there is not an overlay over the previous element anymore; the layers behind QR code modal are transparent. Also, notice the top right- the close button is messed up:

luixxiul · 2016-12-06T02:58:16Z

Thanks @bsclifton for reviewing! I'm fixing both :-)

@bradleyrichter

1. Refactored about:preferences#payments (titleBar and walletBar) - Partly addresses #5494 2. Applied vertical-top to the right element - Fixes #5962 3. Refactored dialog on about:preferences#payments (l10n) - Closes #6007 4. Refactored Brave wallet modal dialog (part 1) - Fixes #5785 based on https://github.com/luixxiul/browser-laptop/commit/2327e9eca53eb4a637dd4c3e3a2a1d1f613c2fbc 5. Replaced the button on the modal dialog with the SVG icon Auditors: @bradleyrichter @bsclifton Test Plan 1: 1. Make sure paymentsMessage and walletBar appears at the same place by toggling the switch off/on 2. Set the toggle to on 3. Make sure every button in titleBar and walletBar works 4. Make sure height of the fund select box and account balance box is equal 5. Make sure the loading message and the question mark is aligned center 6. Make sure no button label is wrapped Test Plan 2: make sure the row of the ledger table on about:payments is aligned horizontally Test Plan 3: 1. Disable Coinbase via VPN 2. Make sure every button on the dialog on about:preferences#payments works Test Plan 4: 1. Disable Coinbase via VPN 2. Make sure the grey footer no longer appears on the Add funds dialog Test Plan 5: 1. Make sure the top right button on the modal dialog works

@luixxiul

Fixes #5951 (was broke with 46ee335) Auditors: @luixxiul Test Plan: 1. Window 1: run `npm run watch-all` 2. Window 2: run `npm run uitest -- --grep="Payment Panel" 3. Watch tests pass

bsclifton · 2016-12-06T09:06:12Z

LGTM! 😄

@jkup, I know you're in the ledger code now- did you want to try this out too? If so (and it looks good), let's merge it 😄

bsclifton · 2016-12-06T09:10:02Z

@luixxiul if you do fix the button alignment for #6039 (comment), can you please do as a follow-up (in a new PR)? thanks 😄

luixxiul · 2016-12-06T09:14:35Z

@bsclifton thanks for reviewing! I will address it with the next PR (here is the plan: #6009 (comment))

bsclifton · 2016-12-06T16:19:25Z

Merging! Thanks, Suguru! 😄

@bsclifton

Resolves #7501 #7380 #6364 Auditors: @bsclifton @cezaraugusto Test plan: - everything should work the same as was before the chage

luixxiul added feature/rewards refactoring labels Dec 3, 2016

luixxiul added this to the 0.13.0 milestone Dec 3, 2016

luixxiul added the QA/test-plan-specified label Dec 3, 2016

luixxiul mentioned this pull request Dec 4, 2016

Refactoring dialog on about:preferences#payments (l10n) #6008

Closed

4 tasks

luixxiul changed the title ~~Refactor about:preferences#payments (titleBar and walletBar)~~ Refactor about:preferences#payments Dec 4, 2016

luixxiul changed the title ~~Refactor about:preferences#payments~~ Refactor about:preferences#payments (part 1) Dec 4, 2016

luixxiul assigned bbondy and bradleyrichter Dec 4, 2016

luixxiul commented Dec 4, 2016

View reviewed changes

willy-b mentioned this pull request Dec 4, 2016

When Modal Overlays are layered, only bottom layer has a dark background (fix #4808) #4820

Merged

3 tasks

luixxiul commented Dec 4, 2016

View reviewed changes

luixxiul modified the milestones: 0.13.1, 0.13.0 Dec 4, 2016

luixxiul commented Dec 4, 2016

View reviewed changes

bsclifton self-assigned this Dec 4, 2016

bsclifton mentioned this pull request Dec 5, 2016

Many Payments Panel tests failing #5951

Closed

luixxiul changed the title ~~Refactor about:preferences#payments (part 1)~~ [WiP] Refactor about:preferences#payments (part 1) Dec 6, 2016

Suguru Hirahara and others added 2 commits December 6, 2016 12:47

Fix broken tests (selector related)

9bdb37a

Fixes #5951 (was broke with 46ee335) Auditors: @luixxiul Test Plan: 1. Window 1: run `npm run watch-all` 2. Window 2: run `npm run uitest -- --grep="Payment Panel" 3. Watch tests pass

luixxiul added the ready-for-merge label Dec 6, 2016

bsclifton mentioned this pull request Dec 6, 2016

Gray footer which contains nothing on the QR code overlay window on about:preferences#payments #5785

Closed

bsclifton mentioned this pull request Dec 6, 2016

Addfunds dialog needs a little margin #5799

Closed

bsclifton merged commit 6545307 into brave:master Dec 6, 2016

This was referenced Dec 6, 2016

Refactor about:preferences#payments followups #6047

Merged

Transfer funds button needs margin-bottom #6048

Closed

luixxiul deleted the refactoring-payment branch December 7, 2016 08:47

luixxiul mentioned this pull request Dec 7, 2016

View payment history link needs padding #4867

Closed

luixxiul removed the ready-for-merge label Dec 7, 2016

This was referenced Dec 8, 2016

the buttons on about:preferences#payments need more paddings #6081

Closed

Funds FAQ icon is underlined on hover #6083

Closed

luixxiul changed the title ~~Refactor about:preferences#payments (part 1)~~ Refactor about:preferences#payments Dec 14, 2016

This was referenced Dec 20, 2016

Manual tests for Windows ia-32 0.13.0 Preview 6 #6335

Closed

Manual tests for Linux Mint 0.13.0 Preview 6 #6337

Closed

Manual tests for Windows x64 0.13.0 Preview 6 #6334

Closed

Manual tests for OS X 0.13.0 Preview 6 #6332

Closed

luixxiul added the QA/checked-Win32 label Dec 20, 2016

luixxiul unassigned bbondy, bsclifton and bradleyrichter Jan 12, 2017

luixxiul mentioned this pull request Jan 16, 2017

The row of the table on about:preferences#payments is not completely aligned #5962

Closed

luixxiul removed QA/checked-Win32 QA/test-plan-specified labels Jan 16, 2017

luixxiul mentioned this pull request Jan 16, 2017

Tracking: Replaced the button on the modal dialog with the SVG icon #6676

Closed

luixxiul referenced this pull request Mar 25, 2017

Refactor of paymentTab (#7481)

0bfceeb

Resolves #7501 #7380 #6364 Auditors: @bsclifton @cezaraugusto Test plan: - everything should work the same as was before the chage

luixxiul mentioned this pull request May 5, 2017

Remove paymentStyles.margin.bar, margin.barItem, padding.bar #8710

Merged

4 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Refactor about:preferences#payments #6009

Refactor about:preferences#payments #6009

luixxiul commented Dec 3, 2016 •

edited

Loading

luixxiul Dec 4, 2016 •

edited

Loading

luixxiul Dec 4, 2016

luixxiul Dec 4, 2016

luixxiul Dec 4, 2016

luixxiul Dec 4, 2016

luixxiul Dec 4, 2016 •

edited

Loading

luixxiul Dec 4, 2016 •

edited

Loading

luixxiul Dec 4, 2016

bsclifton commented Dec 4, 2016

bsclifton commented Dec 5, 2016

luixxiul commented Dec 6, 2016

bsclifton commented Dec 6, 2016

bsclifton commented Dec 6, 2016

luixxiul commented Dec 6, 2016

bsclifton commented Dec 6, 2016

Refactor about:preferences#payments #6009

Refactor about:preferences#payments #6009

Conversation

luixxiul commented Dec 3, 2016 • edited Loading

luixxiul Dec 4, 2016 • edited Loading

Choose a reason for hiding this comment

luixxiul Dec 4, 2016

Choose a reason for hiding this comment

luixxiul Dec 4, 2016

Choose a reason for hiding this comment

luixxiul Dec 4, 2016

Choose a reason for hiding this comment

luixxiul Dec 4, 2016

Choose a reason for hiding this comment

luixxiul Dec 4, 2016 • edited Loading

Choose a reason for hiding this comment

luixxiul Dec 4, 2016 • edited Loading

Choose a reason for hiding this comment

luixxiul Dec 4, 2016

Choose a reason for hiding this comment

bsclifton commented Dec 4, 2016

bsclifton commented Dec 5, 2016

luixxiul commented Dec 6, 2016

bsclifton commented Dec 6, 2016

bsclifton commented Dec 6, 2016

luixxiul commented Dec 6, 2016

bsclifton commented Dec 6, 2016

luixxiul commented Dec 3, 2016 •

edited

Loading

luixxiul Dec 4, 2016 •

edited

Loading

luixxiul Dec 4, 2016 •

edited

Loading

luixxiul Dec 4, 2016 •

edited

Loading